-
Notifications
You must be signed in to change notification settings - Fork 105
fix: increase gas multiple on each resend attempt #746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| !overrides?.maxPriorityFeePerGas | ||
| ) { | ||
| populatedTransaction.maxPriorityFeePerGas *= 2n; | ||
| populatedTransaction.maxPriorityFeePerGas *= gasMultiple; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
maxFeePerGas = maxBaseFeePerGas + maxPriorityFeePerGas
Since maxBaseFeePerGas is uniform across all transactions in a block, setting it significantly above the current base fee provides no benefit. Any excess above the actual block's base fee cannot be collected.
Suggest focusing on increasing maxPriorityFeePerGas instead, as this:
- Directly incentivizes validators to include the transaction
- Can actually be collected (unlike excess base fee)
Consider reducing maxBaseFeePerGas multiplier and redistributing the margin to maxPriorityFeePerGas for more effective fee management.
// For EIP-1559 transactions
if (populatedTransaction.maxPriorityFeePerGas && !overrides?.maxPriorityFeePerGas) {
const originalPriorityFee = populatedTransaction.maxPriorityFeePerGas;
populatedTransaction.maxPriorityFeePerGas *= priorityFeeMultiple;
// Adjust maxFeePerGas to accommodate the increased priority fee
if (populatedTransaction.maxFeePerGas && !overrides?.maxFeePerGas) {
const originalBaseFee = populatedTransaction.maxFeePerGas - originalPriorityFee;
populatedTransaction.maxFeePerGas = originalBaseFee + populatedTransaction.maxPriorityFeePerGas;
}
}
| // reset it to a QueuedTransaction to try again. | ||
| // No transaction hashes means the transaction is not in-flight. | ||
| if ( | ||
| transaction.status === "errored" && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes a wrong assumption where resendCount == 0 means the tx was never sent. ResendCount = 0 just means the tx was never "resent". It may have been sent once.
|
This PR is stale because it has been open for 7 days with no activity. Remove stale label or comment or this PR will be closed in 3 days. |
PR-Codex overview
This PR focuses on enhancing the
sendTransactionWorkerfunctionality by implementing a new method,_updateGasFees, which dynamically adjusts gas fees based on the resend attempt count. It also introduces unit tests to validate the behavior of this method.Detailed summary
_updateGasFeesinsrc/worker/tasks/sendTransactionWorker.tsto compute gas fees based on resend attempts._resendTransactionfunction to utilize_updateGasFees._updateGasFeesinsrc/tests/sendTransactionWorker.test.tsto cover various scenarios, including:gasPricefor legacy transactions.gasPricemultiplier at 10x.maxPriorityFeePerGasandmaxFeePerGasfor EIP-1559 transactions.maxPriorityFeePerGasandmaxFeePerGas.maxPriorityFeePerGasormaxFeePerGasis set.